Fix VS Code Define Step never recovering after a post-open build - #26
Merged
Conversation
Root cause: extensive investigation (raw wire captures, a real dotnet-hosted
LSP client, and a full VS Code Extension Host run) proved the reported
hypothesis wrong — FeatureCodeActionHandler's WorkspaceEdit already uses
DocumentChanges with CreateFile + TextDocumentEdit, and vscode-languageclient
applies that shape correctly (verified against the library's own
CreateFile.is()/TextDocumentEdit.is() type guards and a live apply that
produced the file with correct content).
The real gap: server-side binding discovery (ConnectorBindingRegistryProvider)
reflects over a project's OutputAssemblyPath DLL. If that DLL doesn't exist
when the VS Code extension's ProjectManager sends its first
reqnroll/projectLoaded (e.g. a freshly cloned repo opened before `dotnet
build` has ever run), discovery fails once ("Output assembly not found") and
is never retried. Visual Studio doesn't have this problem because
VsProjectEventMonitor hooks DTE's BuildEvents.OnBuildDone and re-sends every
project after each build; the VS Code extension had no equivalent signal, so
a user who opens, builds, then edits a feature file gets a permanently empty
match cache — matching diagnostics, semantic tokens, and code actions all
silently absent (reproduced end-to-end in a real Extension Host and confirmed
fixed by rebuilding the same scenario against the fix below).
Fix: ProjectManager now watches **/bin/**/*.dll and re-sends
reqnroll/projectLoaded (refreshing outputAssemblyPath) plus the
reqnroll/projectFiles baseline whenever an output assembly is created or
changed, debounced through the existing scheduleResend path. This covers
`dotnet build` from any terminal, a VS Code build task, or C# Dev Kit.
Test plan:
- Added a unit test asserting bin/**/*.dll paths resolve to their owning
project via the existing findOwningProjectFile helper.
- Manually verified end-to-end in a real VS Code Extension Host: with the fix
reverted, a code action requested before a build stayed absent forever
after building; with the fix applied, "Define missing step" appears within
~20s of the build completing, applies correctly, and creates the populated
step-definition file — all without restarting VS Code.
- Full LSP spec suite: 131 passed, 18 pre-existing skips, 0 failures
(dotnet test tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs).
- npm run compile / npm run lint / npm test (VS Code extension unit suite):
all clean, 18 passing.
Closes #2
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The original fix for #2 re-ran MSBuild evaluation and resent the full reqnroll/projectLoaded + reqnroll/projectFiles baseline whenever a project's output DLL was (re)built. That's heavier than the bug needs: the server's WatchedFilesHandler already declares a standard workspace/didChangeWatchedFiles registration for **/bin/**/*.dll and resolves the owning project from the OutputAssemblyPath it received at initial registration (computed from MSBuild properties, correct whether or not the file exists yet) — it just needs to know the file changed, not a full re-evaluation. The client's fallback watcher now sends that same standard LSP notification directly instead of a heavier resend, landing on the server's existing handler with no new server code and no extra dotnet msbuild invocation per rebuild.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WorkspaceEditshape (flatchangesmap instead ofdocumentChangeswithCreateFile) — was tested and disproven:FeatureCodeActionHandleralready sends the correctdocumentChanges/CreateFile/TextDocumentEditshape, and it round-trips correctly throughvscode-languageclient's real type guards and apply logic (verified with a raw wire capture, a standalone Node driver using the actualvscode-jsonrpc/vscode-languageserver-typespackages against the real server, and a full VS Code Extension Host run that created the file with correct content end-to-end).ConnectorBindingRegistryProvider) reflects over a project'sOutputAssemblyPathDLL. If that DLL doesn't exist yet when the VS Code extension'sProjectManagersends its firstreqnroll/projectLoaded(e.g. a freshly cloned repo opened before ever runningdotnet build), discovery fails once ("Output assembly not found") and is never retried. Visual Studio doesn't hit this becauseVsProjectEventMonitorhooks DTE'sBuildEvents.OnBuildDoneand re-sends every project after each build; the VS Code extension had no equivalent, so a user who opens the project, builds it, and then edits a feature file ends up with a permanently empty match cache — diagnostics, semantic tokens, and code actions for undefined steps all silently absent.Revised fix (was: full
reqnroll/projectLoadedresend)The server already declares a standard
workspace/didChangeWatchedFilesregistration for**/bin/**/*.dll(WatchedFilesHandler.cs) specifically to retry discovery once the output assembly appears, resolving the owning project from theOutputAssemblyPathit was given at initial registration (that path is computed from MSBuild properties and is correct even before the file exists). Whether each IDE's LSP client actually delivers those dynamically registered watched-file events reliably is an open design question (Q9 indocs/LSP-IDE-Support-Open-Questions.md) — VS Code'sfiles.watcherExcludecommonly excludesbin//obj/from the file watching a dynamically-registeredFileSystemWatcherFeaturerelies on, which is the most likely reason this wasn't firing.Rather than working around that by re-running
dotnet msbuildand resending the fullreqnroll/projectLoaded+reqnroll/projectFilesbaseline on every rebuild (the original approach in this PR),ProjectManager's fallback watcher now sends the same standardworkspace/didChangeWatchedFilesnotification directly — landing on the server's existing handler with no new server code and no extra MSBuild invocation per rebuild. This is lighter weight and reuses the server's existing debounce/cancel-and-restart discovery logic instead of duplicating it client-side.A follow-up issue tracks investigating and closing Q9 properly (confirming the exclusion-pattern hypothesis and deciding whether the dynamic-registration path can be made reliable enough to drop this client-side fallback entirely).
Test plan
bin/**/*.dllpaths resolve to their owning project via the existingfindOwningProjectFilehelper.dotnet test tests/LSP/Reqnroll.IdeSupport.LSP.Server.Specs).npm run compile/npm run lint/npm test(VS Code extension unit suite): all clean, 17 passing.🤖 Generated with Claude Code
didChangeWatchedFilesrevision. The automated unit suite passes and the logic change is small/mechanical (same trigger, same server-side handler, different message), but a manual re-run is recommended before merge.